[Instrumentation.AWS] Support .NET 6 and resolve AoT warning #1547
Conversation
| #if NET6_0_OR_GREATER | ||
| [System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessage( | ||
| "Trimming", | ||
| "IL2075", | ||
| Justification = "The reflected properties were already used by the AWS SDK's marshallers so the properties could not have been trimmed.")] | ||
| #endif | ||
| private static void AddRequestSpecificInformation(Activity activity, IRequestContext requestContext, string service) | ||
| { | ||
| if (AWSServiceHelper.ServiceParameterMap.TryGetValue(service, out string parameter)) | ||
| if (AWSServiceHelper.ServiceParameterMap.TryGetValue(service, out var parameter)) | ||
| { | ||
| AmazonWebServiceRequest request = requestContext.OriginalRequest; | ||
|
|
||
| var property = request.GetType().GetProperty(parameter); | ||
| if (property != null) | ||
| try | ||
| { | ||
| if (AWSServiceHelper.ParameterAttributeMap.TryGetValue(parameter, out string attribute)) | ||
| var property = request.GetType().GetProperty(parameter); | ||
| if (property != null) | ||
| { | ||
| activity.SetTag(attribute, property.GetValue(request)); | ||
| if (AWSServiceHelper.ParameterAttributeMap.TryGetValue(parameter, out var attribute)) | ||
| { | ||
| activity.SetTag(attribute, property.GetValue(request)); | ||
| } | ||
| } | ||
| } | ||
| catch (Exception) | ||
| { | ||
| // Guard against any reflection-related exceptions when running in AoT. | ||
| // See https://github.com/open-telemetry/opentelemetry-dotnet-contrib/issues/1543#issuecomment-1907667722. | ||
| } |
There was a problem hiding this comment.
See #1543 (comment).
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1547 +/- ##
==========================================
+ Coverage 73.91% 80.62% +6.70%
==========================================
Files 267 113 -154
Lines 9615 3076 -6539
==========================================
- Hits 7107 2480 -4627
+ Misses 2508 596 -1912
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
FYI I've deployed a native AoT compiled .NET 8 application using this package to EKS and observed no issues related to this change and the native AoT compilation warning is no longer present. |
test/OpenTelemetry.AotCompatibility.TestApp/OpenTelemetry.AotCompatibility.TestApp.csproj
Outdated
Show resolved
Hide resolved
Kielek
added
the
comp:instrumentation.aws
martincostello
force-pushed
the
issue-1543
branch
from
e2f4da8
to
f502130
Compare
Add `net6.0` TFM and fix nullability warnings.
Fix IDE0090 suggestions.
Mark `AWSTracingPipelineHandler` as `sealed` as a hint to the JIT to devirtualize calls.
Resolve AoT warning. Fixes #1543.
Update changelog with .NET 6 and AoT changes.
martincostello
force-pushed
the
issue-1543
branch
from
73c64ae
to
2e68c2f
Compare
|
Could I get an ETA on when this will be merged and when there will be a new release of the packages updated by this PR as well as #1545 and #1541? I've a number of applications where I'd like to consume the changes in as these packages are the blocker for native AoT working in them correctly. Alternatively, if there's a pre-release feed for NuGet packages from your CI I can consume those in the meantime. |
|
Merging as @srprash, agreed that @normj can act as component owner in #1539. There is only some technical requirements to merge mentioned PR. If we speaking about release - please follow process from https://github.com/open-telemetry/opentelemetry-dotnet-contrib/blob/main/CONTRIBUTING.md#how-to-request-for-release-of-package |
Fixes #1543.
Changes
net6.0TFM and fix nullability warnings.AWSTracingPipelineHandlerassealed(e.g. [ResourceDetectors.AWS] mark class as sealed #1510).IDE0090suggestions.CHANGELOG.mdupdated for non-trivial changes